test(multidb): e2e failover tests via per-member RESP proxies - #3952

multidb/e2e/harness_test.go:91

• RestoreAll ignores errors from docker start. If the container is missing (or start fails for any other reason), the test will wait up to 30s in awaitListening and then fail with a less-informative timeout. Handle docker start errors explicitly (at least failing fast on missing containers).

		if err := f.docker("unpause", m.Container); err != nil && isMissingContainer(err) {
			f.t.Fatalf("proxy container %s does not exist — start the stack with `docker compose --profile multidb up -d`: %v", m.Container, err)
		}
		_ = f.docker("start", m.Container)
		f.awaitListening(i, 30*time.Second)

docker-compose.yml:86

• These MultiDB proxy containers are included in the all profile, so make docker.start / make test will now start and bind ports for 3 extra proxies even when MultiDB e2e tests are not being run. If they are only needed for the multidb profile, drop all here to avoid extra containers and host port allocations in the default stack.

    profiles:
      - multidb
      - all

docker-compose.yml:104

• These MultiDB proxy containers are included in the all profile, so make docker.start / make test will now start and bind ports for 3 extra proxies even when MultiDB e2e tests are not being run. If they are only needed for the multidb profile, drop all here to avoid extra containers and host port allocations in the default stack.

    profiles:
      - multidb
      - all

multidb/e2e/scenarios_test.go:196

• publishUntilReceived reads from msgs without checking whether the channel is closed. If the PubSub channel closes (e.g. due to an unexpected subscription teardown), the receive case becomes immediately ready and the loop can spin, masking the real failure behind a timeout.

			select {
			case m := <-msgs:
				if m.Payload == tag {
					return
				}
			case <-time.After(250 * time.Millisecond):
			}

multidb/e2e/harness_test.go:50

• proxyFarm.docker uses exec.Command without a context/timeout, so a stuck Docker CLI call can hang the entire test process until the global go test -timeout fires. Wrap the command in a short CommandContext timeout so failures surface quickly and the suite can't deadlock on a Docker issue.

func (f *proxyFarm) docker(args ...string) error {
	out, err := exec.Command("docker", args...).CombinedOutput()
	if err != nil {
		return fmt.Errorf("docker %v: %v: %s", args, err, out)
	}
	return nil
}

multidb/e2e/scenarios_test.go:97

• The test comment says the client reports temporary unavailability and then permanent after the attempt budget, but the test currently returns as soon as it sees either Temporary or Permanent. This makes the comment misleading (and the scenario name ambiguous) for readers trying to understand what is actually asserted.

// TestEscalationWhenAllMembersDown: with every member stopped the client
// reports temporary unavailability, then permanent after the attempt budget;
// restarting a member during the temporary phase recovers.
// Spec: test_all_databases_unreachable_error + escalation chain.

docker-compose.yml:68

• These MultiDB proxy containers are included in the all profile, so make docker.start / make test will now start and bind ports for 3 extra proxies even when MultiDB e2e tests are not being run. If they are only needed for the multidb profile, drop all here to avoid extra containers and host port allocations in the default stack.

This issue also appears in the following locations of the same file:

• line 84
• line 102

    profiles:
      - multidb
      - all

docker-compose.yml:86

• Same as cae-proxy-db0: including this service in the all profile means it will be started (and bind host ports) for every make docker.start / make test run. Unless you explicitly want that behavior, keep it scoped to the multidb profile only.

    profiles:
      - multidb
      - all

docker-compose.yml:104

• Same as cae-proxy-db0/1: this service being in the all profile makes it come up in the default docker stack, which is unnecessary for non-MultiDB test runs and can cause avoidable port/resource usage.

    profiles:
      - multidb
      - all

docker-compose.yml:68

• These MultiDB proxy containers are included in the all compose profile. Since make docker.start uses --profile all, this will start the MultiDB proxies (and bind ports 17100/18120) during every normal test run, even though the MultiDB e2e suite is env-gated. Keeping these services only under the multidb profile avoids unnecessary containers/port usage for the default workflow.

This issue also appears in the following locations of the same file:

• line 84
• line 102

    profiles:
      - multidb
      - all

Makefile:130

• test.multidb.e2e brings up compose without exporting the same env vars as docker.start (e.g. CLIENT_LIBS_TEST_IMAGE, REDIS_VERSION, RE_CLUSTER, RCE_DOCKER). That means make test.multidb.e2e CLIENT_LIBS_TEST_IMAGE=... (or other overrides) won’t be honored by docker-compose, and the Redis image/tag can silently differ from the rest of the test workflow.

test.multidb.e2e:
	@echo "Running MultiDB e2e tests (per-member proxies)..."
	docker compose --profile multidb up -d
	@E2E_MULTIDB_TESTS=true go test -v -race -count=1 -timeout 15m ./multidb/e2e/... || (docker compose --profile multidb down && exit 1)
	docker compose --profile multidb down

multidb/e2e/scenarios_test.go:196

• If the PubSub channel closes (e.g. due to an underlying connection failure during failover), receiving from msgs yields nil and m.Payload will panic. Failing the test with an explicit error here makes failures easier to diagnose and avoids a nil-pointer panic.

			select {
			case m := <-msgs:
				if m.Payload == tag {
					return
				}

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

docker-compose.yml:90

• Using a mutable :latest tag makes the multidb e2e environment non-reproducible and can introduce CI breakages when the proxy image updates. Consider making the image tag configurable via an env var (with a default) so CI/local runs can pin a known-good version.

  cae-proxy-db2:
    image: redislabs/client-resp-proxy:latest
    container_name: cae-proxy-db2

docker-compose.yml:72

• Using a mutable :latest tag makes the multidb e2e environment non-reproducible and can introduce CI breakages when the proxy image updates. Consider making the image tag configurable via an env var (with a default) so CI/local runs can pin a known-good version.

  cae-proxy-db1:
    image: redislabs/client-resp-proxy:latest
    container_name: cae-proxy-db1

multidb/e2e/harness_test.go:91

• RestoreAll ignores errors from docker start. If Docker is unavailable or the start fails for another real reason, the test will instead wait up to 30s per member and then fail with the generic "never came back up" message. Treat docker start errors as fatal to fail fast with the underlying cause.

		if err := f.docker("unpause", m.Container); err != nil && isMissingContainer(err) {
			f.t.Fatalf("proxy container %s does not exist — start the stack with `docker compose --profile multidb up -d`: %v", m.Container, err)
		}
		_ = f.docker("start", m.Container)
		f.awaitListening(i, 30*time.Second)

multidb/e2e/scenarios_test.go:196

• In publishUntilReceived, receiving from a closed PubSub channel returns a nil *redis.Message; accessing m.Payload will panic and the failure will be hard to diagnose. Capture the receive ok flag (and optionally nil-check) to fail with a clear error instead of panicking.

			select {
			case m := <-msgs:
				if m.Payload == tag {
					return
				}

docker-compose.yml:54

• Using a mutable :latest tag makes the multidb e2e environment non-reproducible and can introduce CI breakages when the proxy image updates. Consider making the image tag configurable via an env var (with a default) so CI/local runs can pin a known-good version.

This issue also appears in the following locations of the same file:

• line 70
• line 88

  cae-proxy-db0:
    image: redislabs/client-resp-proxy:latest
    container_name: cae-proxy-db0

Makefile:128

• test.multidb.e2e runs docker compose without exporting the Makefile variables (REDIS_VERSION / CLIENT_LIBS_TEST_IMAGE / etc.). As a result, overrides like make test.multidb.e2e CLIENT_LIBS_TEST_IMAGE=... won’t affect the compose stack, unlike docker.start/docker.e2e.start which explicitly export these variables.

test.multidb.e2e:
	@echo "Running MultiDB e2e tests (per-member proxies)..."
	docker compose --profile multidb up -d
	@E2E_MULTIDB_TESTS=true go test -v -race -count=1 -timeout 15m ./multidb/e2e/... || (docker compose --profile multidb down && exit 1)

docker-compose.yml:71

• Using the :latest tag makes the test environment non-reproducible and can introduce unexpected breakage when the image is updated upstream. Consider allowing a pinned override via an environment variable (defaulting to the current value) so CI and local runs can lock to a known-good proxy version.

    image: redislabs/client-resp-proxy:latest

docker-compose.yml:89

• Using the :latest tag makes the test environment non-reproducible and can introduce unexpected breakage when the image is updated upstream. Consider allowing a pinned override via an environment variable (defaulting to the current value) so CI and local runs can lock to a known-good proxy version.

    image: redislabs/client-resp-proxy:latest

multidb/e2e/harness_test.go:36

• Using localhost here makes the tests depend on name resolution (it may prefer ::1 on IPv6-enabled hosts). Docker port mappings are not always bound on IPv6, so dialing localhost can fail even though 127.0.0.1 would work. Use an explicit IPv4 loopback address for deterministic connectivity.

		members: []memberProxy{
			{Container: "cae-proxy-db0", Addr: "localhost:17100"},
			{Container: "cae-proxy-db1", Addr: "localhost:17101"},
			{Container: "cae-proxy-db2", Addr: "localhost:17102"},

Makefile:128

• Other docker bring-up targets use --quiet-pull (e.g. docker.start and docker.e2e.start) to ensure required images are pulled before running tests. Adding it here keeps behavior consistent and avoids failures when the proxy image isn't present locally.

	@echo "Running MultiDB e2e tests (per-member proxies)..."
	docker compose --profile multidb up -d
	@E2E_MULTIDB_TESTS=true go test -v -race -count=1 -timeout 15m ./multidb/e2e/... || (docker compose --profile multidb down && exit 1)

docker-compose.yml:53

• Using the :latest tag makes the test environment non-reproducible and can introduce unexpected breakage when the image is updated upstream. Consider allowing a pinned override via an environment variable (defaulting to the current value) so CI and local runs can lock to a known-good proxy version.

This issue also appears in the following locations of the same file:

• line 71
• line 89

    image: redislabs/client-resp-proxy:latest

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

multidb/e2e/scenarios_test.go:294

• Same as the non-pattern PubSub case: if msgs is closed, the receive yields nil and m.Payload will panic. Add a nil guard so failures report a clear assertion instead of a panic.

			case m := <-msgs:
				if m.Payload == tag {
					return
				}

multidb/e2e/scenarios_test.go:246

• msgs is a <-chan *redis.Message; if the channel gets closed (e.g. subscription closes due to an unexpected reconnect/teardown), receiving yields nil and m.Payload will panic. Handle a nil message and fail the test with a clear error instead of panicking.

This issue also appears on line 291 of the same file.

			case m := <-msgs:
				if m.Payload == tag {
					return
				}

Makefile:130

• The proxy readiness loop uses curl -s, which exits 0 even for HTTP error responses (e.g. 404/500), so the loop can succeed while the API endpoint is unhealthy/miswired. Use -f (and optionally -S) so only a successful HTTP response breaks the loop.

		timeout 30 bash -c "until curl -s http://127.0.0.1:$$port/stats > /dev/null; do sleep 1; done" || (docker compose --profile multidb down && exit 1); \

.github/workflows/test-e2e.yml:93

• The readiness probe uses curl -s, which treats HTTP 404/500 as success and can let the job proceed even if /stats is returning an error. Use curl -f (and -S) so the loop only exits on a healthy HTTP response.

            timeout 30 bash -c "until curl -s http://127.0.0.1:$port/stats > /dev/null; do sleep 1; done"

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Makefile:131

• test.multidb.e2e's readiness loop relies on the external timeout binary. Unlike go test -timeout, timeout isn't available by default on some dev environments (notably macOS), so make test.multidb.e2e can fail before running any tests. You can implement the same 30s bound port-wait using POSIX shell + date (and also make the curl check fail on HTTP errors via -f).

	@for port in 18120 18121 18122; do \
		timeout 30 bash -c "until curl -s http://127.0.0.1:$$port/stats > /dev/null; do sleep 1; done" || { docker compose --profile multidb down; exit 1; }; \
	done

docker-compose.yml:86

• Same as cae-proxy-db0: including this service in the all profile makes it start during default make docker.start / make test runs, even though MultiDB e2e is meant to be opt-in. Consider keeping it only in the multidb profile.

    profiles:
      - multidb
      - all

docker-compose.yml:104

• Same as cae-proxy-db0/1: including this service in the all profile makes it start during default make docker.start / make test runs, even though MultiDB e2e is meant to be opt-in. Consider keeping it only in the multidb profile.

    profiles:
      - multidb
      - all

docker-compose.yml:68

• These per-member MultiDB proxy containers are included in the all profile, which is what make docker.start / make test uses (docker compose --profile all up). That means normal test runs will now always start (and bind ports for) these extra proxies even when the MultiDB e2e suite is not being run. If the intent is for MultiDB E2E to remain opt-in, keep these services only under the multidb profile.

This issue also appears in the following locations of the same file:

• line 84
• line 102

    profiles:
      - multidb
      - all

Makefile:131

• The readiness probe uses curl -s which returns success even for HTTP error status codes (e.g. 404/500). That can let the target be considered "ready" while the proxy API is actually failing, and then the tests fail later with less direct errors. Use -f (and optionally -S) so only a successful HTTP response counts as ready.

	@for port in 18120 18121 18122; do \
		timeout 30 bash -c "until curl -s http://127.0.0.1:$$port/stats > /dev/null; do sleep 1; done" || { docker compose --profile multidb down; exit 1; }; \

.github/workflows/test-e2e.yml:94

• The readiness probe uses curl -s, which still exits 0 on HTTP error status codes and can treat an unhealthy proxy API as "ready". Use -f (and optionally -S) so only a successful HTTP response satisfies the loop.

          for port in 18120 18121 18122; do
            timeout 30 bash -c "until curl -s http://127.0.0.1:$port/stats > /dev/null; do sleep 1; done"
          done

multidb/e2e/harness_test.go:16

• The comment says each member endpoint is a "cae-resp-proxy" container, but the harness actually uses the per-member containers (cae-proxy-db0/1/2). This makes the harness documentation misleading when debugging test failures.

// memberProxy is one MultiDB member endpoint: a cae-resp-proxy container
// fronting the shared target Redis.

Makefile:132

• The Makefile target waits for the proxy /stats endpoints but does not wait for Redis itself to be ready. The CI workflow explicitly pings Redis first; without the Redis readiness check, local runs can start tests while Redis is still booting, increasing flake risk.

	@echo "Running MultiDB e2e tests (per-member proxies)..."
	docker compose --profile multidb up -d || (docker compose --profile multidb down && exit 1)
	@echo "Waiting for the per-member proxies to be ready..."
	@for port in 18120 18121 18122; do \
		timeout 30 bash -c "until curl -s http://127.0.0.1:$$port/stats > /dev/null; do sleep 1; done" || { docker compose --profile multidb down; exit 1; }; \
	done
	@E2E_MULTIDB_TESTS=true go test -v -race -count=1 -timeout 15m ./multidb/e2e/... || (docker compose --profile multidb down && exit 1)